-
-
Notifications
You must be signed in to change notification settings - Fork 4
[Ready] Changes centering calculations for modals #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
To test you should be able to just open the index.html file in the repository and click any modal button there. I think we're going to have to expressly include a container though, styling the modals normal parent will probably break some other styling. It's probably best if we have a container that is explicitly set as 100% width and 100% height to center in. |
|
Alright, tested it and everything and it's good. Let me know if anything else needs doing (differently). |
|
I'm closing this in favour of #13 where I've done basically the same thing by moving the existing overlay around and using that for positioning instead of creating a whole new element. Thanks for the help! |
|
No problem. I actually tried to move it to the existing overlay, but I couldn't figure out how to resolve the resulting opacity issue. I'll look at your implementation to see how it was done. |
|
I passed the opacity into an |
|
Yep, just saw the code now. I thought about that, but then I realized I'd have to figure out how exactly you handled the fade out. I also see you refactored the relevant bits significantly, even changing the overlay from a class to an ID, which isn't something I could have done. |
|
Yeah, I ended up just breaking the fadeOut in favor of CSS Transitions. jQuery was getting some pretty poor framerates over on elementary. |
Fixes #7 and elementary/website#1255
I don't know how to test this so please tell me how to do so, or test it yourself and confirm the proposed changes are sufficient. Thank you!